feat(config): extensions section, capabilities bindings, and config-level validation#2538
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2538 +/- ##
========================================
Coverage 88.37% 88.38%
========================================
Files 622 622
Lines 230058 230531 +473
========================================
+ Hits 203318 203746 +428
- Misses 26216 26261 +45
Partials 524 524
🚀 New features to boost your workflow:
|
gouslu
force-pushed
the
gouslu/extension-system-pr1-config
branch
2 times, most recently
from
4eec6cc to
18e9f65
Compare
gouslu
force-pushed
the
gouslu/extension-system-pr1-config
branch
from
18e9f65 to
a520a65
Compare
|
The architecture doc link doesn't work. Can you please fix that? I'd like to understand what this is being built against. |
|
I believe this is ready to review because this PR1 is consistent with the original extensions design document. If there are objections to #2510, let them surface now! I will merge that PR before I merge this one, but both are ready to review. |
- Fixed imports in crates/config/src/node.rs (merged both HEAD and main branches) - Fixed Extension variant in node.rs match expressions - Fixed Extension variant in crates/controller/src/startup.rs - Removed duplicate dead code functions from src/main.rs - Cleaned up unused imports - All tests pass, rustfmt formatted, clippy clean
gouslu
force-pushed
the
gouslu/extension-system-pr1-config
branch
from
31e0c61 to
f309fd6
Compare
| skip_serializing_if = "HashMap::is_empty", | ||
| deserialize_with = "deserialize_no_dup_keys" | ||
| )] | ||
| pub capabilities: HashMap<CapabilityId, NodeId>, |
There was a problem hiding this comment.
This enforces one extension per capability per node. Is this intentional? The design doc doesn't state this explicitly.
If an exporter needs header_setter functionality from two independent extensions (e.g., one for correlation headers, one for custom metadata headers), the only workaround would be for the user to write a custom extension that combines both. That seems like a heavy ask for what might be a common use case.
There was a problem hiding this comment.
I noticed this, at some point. I feel that if there are more than one capability, we should follow the Collectr's naming convention and use names like bearer_token/left bearer_token/right if you need more than one. I figure we should tackle this when it happens.
There was a problem hiding this comment.
I'm okay with tackling this later. However, one thing to keep in mind: the design doc states that capabilities are "known by the engine version and validated during configuration loading," and that external extensions cannot introduce new capability interfaces. This is different from the Go Collector, where any extension can expose any interface and components look them up by ID at runtime.
So bearer_token/left and bearer_token/right would each need to be pre-registered as known capabilities in the engine core. It's not purely a user naming convention like it would be in the Go Collector. Each new variant requires an engine core change, so this may not be as straightforward to address when the time comes. Just wanted to flag this so it's not overlooked.
There was a problem hiding this comment.
Depends on how you resolve them. /left and /right might just mean that there are 2 different instances and that you get provided with a map hat has both in it or something like that. What would need to be known is the capability id basically, same as today.
640e709
4 participants
Change Summary
This is PR 1 based on #2510.
Add config parsing for pipeline extensions — the first building block of the extension system (architecture doc).
Extensions are standalone pipeline components that provide shared capabilities (auth, storage, etc.) to data-path nodes. This PR adds config-level support only — no runtime behavior. Extensions are parsed and validated but not created or started.
Config changes:
extensions:section in pipeline config, separate fromnodes:capabilities:field on nodes for binding capabilities to extension instancesNodeKind::Extensionvariant for URN parsing (e.g.,urn:otel:extension:auth)ExtensionConfigstruct in engine configPipelineConfigBuilder::add_extension()for programmatic config constructionValidations added:
extensions:sectioncapabilities:bindings (they provide capabilities, not consume them)nodes:section →ExtensionInNodesSectionerrorextensions:at group level rejected by serde (deny_unknown_fields)Example config:
What issue does this PR close?
How are these changes tested?
13 new config tests covering:
All 168 config tests pass. Example config YAML added to
configs/.Are there any user-facing changes?
Yes — pipeline YAML configs now support
extensions:andcapabilities:sections. These are optional and have no runtime effect in this PR.